Improve flexibility for publishing options #2964
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #2964 +/- ##
=======================================
- Coverage 88.7% 88.6% -0.2%
=======================================
Files 89 89
Lines 11023 11040 +17
=======================================
+ Hits 9785 9788 +3
- Misses 1238 1252 +14 ☔ View full report in Codecov by Sentry. |
|
Please take another look, I have retained the original behavior of copying to GCS after the ETL finishes, regardless of the outcome. |
|
I have merged |
| @@ -109,6 +116,9 @@ if [[ $ETL_SUCCESS == 0 ]]; then | |||
| ETL_SUCCESS=${PIPESTATUS[0]} | |||
There was a problem hiding this comment.
Probably want to remove the success file here prior to distribution.
There was a problem hiding this comment.
If we replace success file with, say, json file that has some runtime statistics from how the data was generated, would we want to include it in distributed files, or also not?
There was a problem hiding this comment.
I think a little build metadata file would be worth including. We're distributing the pudl-etl.log file. It would kind of be a structured accessory to that output.
| @@ -83,6 +85,11 @@ jobs: | |||
| - name: Set up Cloud SDK | |||
| uses: google-github-actions/setup-gcloud@v1 | |||
|
|
|||
| - name: Determine commit information | |||
| run: |- | |||
There was a problem hiding this comment.
TIL about the block chomping operator. Does GHA complain when there's an extra newline at the end here?
| @@ -109,6 +120,9 @@ if [[ $ETL_SUCCESS == 0 ]]; then | |||
| ETL_SUCCESS=${PIPESTATUS[0]} | |||
|
|
|||
| # Dump outputs to s3 bucket if branch is dev or build was triggered by a tag | |||
| # TODO: this behavior should be controlled by on/off switch here and this logic | |||
There was a problem hiding this comment.
Agree - having the action just pass in a switch would be nice. I think at some point we should replace this whole nightly build harness with a Python script that's more robust and that would be a nice time to fix this too.
This change slightly improves upon the flexibility of publishing outputs to remote destinations.
PUDL_GCS_OUTPUTenv variable is introduced togcp_pudl_etl.shrunner script to possibly override the destination used to publish stuff.PUDL_PUBLISH_DESTINATIONSenv variable to immediately ship outputs after ETL finishes.Option (2) should eventually replace the logic contained within (1), once we integrate data validation into the ETL or decouple it and do it later on.